Conversation
7a428ae to
b69d0dd
Compare
…ountStore with fail-closed isolation Ports JS @adcp/sdk@6.7's createTenantStore. The headline 6.7 helper: an AccountStore with a per-entry tenant gate baked in so cross-tenant entries never reach adopter code on upsert/sync_governance. Fail-closed: when resolve_from_auth(ctx) returns None (unauthenticated or unregistered principal), every entry rejects with PERMISSION_DENIED and list() returns []. resolve() also enforces the gate on Path-1 (operator-routed) calls — cross-tenant refs return None, hiding the existence of accounts the caller can't see. Immutability: gate methods are class-level and the store class uses __slots__, so adopter code that tries store.upsert = custom_handler gets AttributeError instead of silently bypassing isolation. Closes #457. Part of #452.
…ck isolation Address code-review BLOCKER and security-review SHOULD-FIX items: - resolve(ref, ctx) → resolve(ref, auth_info=None) matching the AccountStore Protocol. Adopter resolve_by_ref(ref, ctx) callback unchanged — ResolveContext is synthesized inside the public method. - Per-entry try/except inside upsert and sync_governance: a single callback exception no longer poisons the rest of the batch. Failed entries surface as PERMISSION_DENIED on the wire (no exception text leaked) and are logged server-side via logger.warning(exc_info=True). - list now catches tenant_to_account exceptions and returns [] — honoring the docstring's MUST-NOT-RAISE contract. - resolve Path 1 catches resolve_by_ref exceptions, returns None + logs WARNING (so a single adopter raise doesn't 500 the request). - Added tests for: AccountStore isinstance check, dispatcher kwarg call shape, four callback-raises cases. - Class-level immutability docstring note added (instance immutability is enforced via __slots__; class-level monkey-patching is possible but the leading-underscore + non-export keep _TenantStore out of adopter code paths).
b69d0dd to
8199270
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of JS
@adcp/sdk@6.7'screateTenantStore— the headline 6.7 helper. Builds an opinionated multi-tenantAccountStorewith the per-entry tenant gate baked into the framework so cross-tenant entries never reach adopter code.Security semantics
resolve_from_auth(ctx)returnsNone(unauthenticated / unregistered principal), everyupsertandsync_governanceentry rejects withPERMISSION_DENIED.resolvereturnsNone.listreturns[].upsert_row/sync_governance_rowcallbacks. Mixed batches partition cleanly into pass /PERMISSION_DENIED/ACCOUNT_NOT_FOUND.__slots__. Adopter code that triesstore.upsert = custom_handlergetsAttributeErrorinstead of silently bypassing isolation. The Python equivalent of JS'sObject.defineProperty(... writable: false).Design decisions / divergences from JS
Tenanttotenant_id: str. The JS API takes a genericTTenantvalue plus atenantId(tenant): stringprojection. The Python adaptation collapses this to a single string identity since adopters typically denormalize the owning tenant onto theAccountitself.tenant_id(account)projects from Account;tenant_to_account(tenant_id)is the inverse. Same security semantics, simpler shape.resolve()also enforces the gate. JS only gatesupsert/syncGovernance; Python tightens this to gateresolvetoo (per the issue spec). Cross-tenant refs returnNone, hiding the existence of accounts the caller can't see.list()is generated. JS deliberately omitslistfrom the helper because cursor pagination + multi-tenant filter shapes don't fit a one-liner. Python provides a single-tenant projection (returns[tenant_to_account(auth_tenant)]) since the prompt specified it; adopters needing pagination compose by wrapping the returned store.upsert_row/sync_governance_roware optional (matches JS). Without an adopter hook, authorized rows pass withaction='unchanged'(no-op acknowledgment) rather than 501. Cross-tenant rejection still applies.Tests cover
None), unknown ref, unauthenticated, Path-2 (no ref → auth-derived account)ACCOUNT_NOT_FOUND, fail-closed for unregistered + unauthenticated, mixed-batch partitioning, no-hook no-op behavior[], unauthenticated →[]upsert/sync_governance/list/resolve(all raiseAttributeError)23 tests, all green. Full regression suite (3383 tests) stays green.
Closes #457. Part of #452.
🤖 Generated with Claude Code